Skip to content

fix(cli): surface verbose error details#416

Merged
moncher-dev merged 1 commit into
mainfrom
feat/396-verbose-errors
Jun 24, 2026
Merged

fix(cli): surface verbose error details#416
moncher-dev merged 1 commit into
mainfrom
feat/396-verbose-errors

Conversation

@moncher-dev

@moncher-dev moncher-dev commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

TL;DR

  • Makes -v / --verbose produce stack traces and cause chains for top-level CLI and orchestrator failures.
  • Keeps default error output as the existing one-line message.
  • Bridges global CLI verbose mode into repo start orchestrator verbose logging, including daemon child startup diagnostics.

변경 지점 다이어그램

@gh-symphony/core observability
  └─ formatErrorForTerminal(error, { verbose })
      ├─ packages/cli top-level catch
      └─ packages/orchestrator top-level catch

CLI global -v
  ├─ top-level crash formatting
  └─ repo start
      ├─ foreground orchestrator logLevel=verbose
      └─ daemon child argv includes --verbose + --log-level verbose

여기부터 보세요

  • packages/core/src/observability/error-format.ts — shared terminal error formatter, verbose flag detection, and circular cause protection.
  • packages/cli/src/index.ts — CLI binary catch now uses verbose formatting.
  • packages/orchestrator/src/index.ts — orchestrator binary catch uses verbose formatting and accepts -v / --verbose.
  • packages/cli/src/commands/start.ts — global CLI verbose flag maps to orchestrator verbose logs and daemon child diagnostics.

위험 & 롤백

  • Risk: verbose output can expose more local stack detail when explicitly requested. Default non-verbose output is unchanged.
  • Rollback: revert the formatter wiring and help text changes; no data migration or runtime state change is involved.

변경 파일

  • .changeset/verbose-error-causes.md
  • packages/core/src/observability/error-format.ts
  • packages/core/src/observability/error-format.test.ts
  • packages/core/src/observability/index.ts
  • packages/cli/src/index.ts
  • packages/cli/src/commands/help.ts
  • packages/cli/src/commands/__snapshots__/help.test.ts.snap
  • packages/cli/src/commands/start.ts
  • packages/cli/src/commands/start.test.ts
  • packages/orchestrator/src/index.ts
  • packages/orchestrator/src/index.test.ts

Evidence

  • pnpm --filter @gh-symphony/core test -- error-format — pass
  • pnpm --filter @gh-symphony/cli test -- start — pass
  • pnpm exec vitest run packages/core/src/observability/error-format.test.ts packages/orchestrator/src/index.test.ts packages/cli/src/commands/start.test.ts packages/cli/src/commands/help.test.ts — pass
  • pnpm lint — pass
  • pnpm test — pass
  • pnpm typecheck — pass
  • pnpm build — pass
  • Changeset: .changeset/verbose-error-causes.md
  • Docker E2E: N/A, no orchestration dispatch/worker lifecycle/tracker integration behavior changed.

Issues — Closed #396

Closes #396

머지 후/사람 확인

  • Confirm gh-symphony -v ... shows stack/cause details for an uncaught CLI failure.
  • Confirm default non-verbose failure output remains a single-line message.

@moncher-dev moncher-dev marked this pull request as ready for review June 24, 2026 16:14
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 39.13% 28578 / 73018
🔵 Statements 39.13% 28578 / 73018
🔵 Functions 71.63% 1399 / 1953
🔵 Branches 76.49% 5869 / 7672
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/cli/src/index.ts 80.49% 82.25% 72% 80.49% 123-124, 138-140, 148-150, 152-153, 176-177, 192-193, 254-259, 280-292, 302-306, 321-329, 342-349, 373-381, 390-391, 450-451, 495-499, 510-515, 642-646, 654-655, 661-666, 677-682, 688-693, 712-717, 738-739, 753-754, 757-759, 765-773
packages/cli/src/commands/help.ts 97.25% 100% 80% 97.25% 199-203
packages/cli/src/commands/start.ts 79.97% 73.23% 97.29% 79.97% 102-103, 125-127, 132, 135-137, 145-151, 167-168, 184-185, 192-193, 207-208, 215-216, 340-349, 351-353, 399-400, 403-404, 408-410, 416-422, 436-441, 445-450, 454-456, 460-465, 468-474, 479-480, 484-487, 506-507, 513, 515, 590-591, 626-627, 649-654, 675-677, 679-684, 695-700, 782-795, 807-808, 827-828, 903, 907-908, 919-920, 939-963, 989-995, 1001-1005, 1010-1014, 1019-1023, 1056, 1058
packages/core/src/observability/error-format.ts 94.73% 73.33% 100% 94.73% 45-46
packages/core/src/observability/index.ts 100% 100% 100% 100%
packages/orchestrator/src/index.ts 69.87% 74.6% 63.63% 69.87% 85-89, 111, 117-118, 122-123, 133-136, 176-185, 187-190, 192-195, 197, 201-227, 249-250, 263-265, 285, 304-322
Generated in workflow #306 for commit e1ae786 by the Vitest Coverage Report Action

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d71d726ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

options,
projectId,
parsed.logLevel,
parsed.logLevel ?? (options.verbose ? "verbose" : undefined),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve verbose diagnostics for daemon child

In the daemon path, gh-symphony -v repo start --daemon converts the parent verbose flag only into --log-level verbose for the detached child. The child is another CLI process whose top-level catch enables stack/cause output only when hasVerboseFlag(process.argv.slice(2)) sees -v/--verbose, so uncaught startup failures in the daemon log still get the old single-line message even though the user requested verbose diagnostics. Pass the verbose flag through separately, or make the child diagnostic check recognize this propagated state.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5049ad9: startDaemon now passes --verbose to the child CLI when the parent global verbose option is set, alongside --log-level verbose. Added start.test.ts coverage asserting the daemon child argv includes both flags.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves CLI/orchestrator failure diagnostics by introducing a shared terminal error formatter in @gh-symphony/core and wiring the existing -v/--verbose flag to show stack traces and cause chains, while preserving the current one-line default output. This primarily affects the Observability layer (error formatting/output) and its wiring at CLI entrypoints.

Changes:

  • Add a shared formatErrorForTerminal(error, { verbose }) utility (plus hasVerboseFlag) in core observability, with tests.
  • Update CLI and orchestrator top-level catch handlers to use the shared formatter and respect verbose mode.
  • Bridge CLI global verbose into repo start by mapping it to orchestrator logLevel=verbose and documenting the flag in help output.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/core/src/observability/error-format.ts New shared terminal error formatter and verbose flag detection.
packages/core/src/observability/error-format.test.ts Unit tests for verbose/non-verbose terminal formatting behavior.
packages/core/src/observability/index.ts Re-export new error formatting utilities from observability barrel.
packages/cli/src/index.ts Use shared formatter in top-level CLI crash handler; improve verbose option help text.
packages/cli/src/commands/help.ts Update --verbose/-v help description to mention stack traces.
packages/cli/src/commands/snapshots/help.test.ts.snap Snapshot updates for the revised verbose help text.
packages/cli/src/commands/start.ts Map global CLI verbose to orchestrator verbose log level for repo start.
packages/cli/src/commands/start.test.ts Add coverage ensuring global verbose maps to orchestrator verbose logs.
packages/orchestrator/src/index.ts Use shared formatter in top-level catch; accept -v/--verbose as logLevel=verbose.
packages/orchestrator/src/index.test.ts Add coverage ensuring -v is treated as verbose log level.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +17 to +32
const lines = [formatSingleError(error)];
const seenCauses = new Set<object>();
let cause = resolveCause(error);

while (cause !== undefined) {
if (typeof cause === "object" && cause !== null) {
if (seenCauses.has(cause)) {
lines.push("Caused by: [Circular cause]");
break;
}
seenCauses.add(cause);
}

lines.push(`Caused by: ${formatSingleError(cause)}`);
cause = resolveCause(cause);
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5049ad9: formatErrorForTerminal now seeds the seen-cause set with the top-level error before walking cause, so an A -> B -> A cycle emits [Circular cause] instead of printing the top-level error twice.

Comment on lines +30 to +35
it("detects both verbose flags", () => {
expect(hasVerboseFlag(["repo", "start", "--verbose"])).toBe(true);
expect(hasVerboseFlag(["-v", "repo", "start"])).toBe(true);
expect(hasVerboseFlag(["repo", "start"])).toBe(false);
});
});

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 5049ad9: error-format.test.ts now covers a circular cause chain returning to the top-level error and asserts the formatter emits [Circular cause] without repeating the top-level stack.

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 완성도 검증 결과 — Approve

원본 이슈 #396의 수용 조건과 PR 변경 내용을 검증했고, 빌드/테스트/스모크 테스트를 직접 수행했습니다.

수용 조건 충족 여부 (#396)

  • -v/--verbose가 더 이상 no-op이 아님 — top-level catch에서 stack 및 cause 체인을 출력
  • ✅ CLI top-level catch가 argv를 스캔해 verbose 적용 (packages/cli/src/index.ts)
  • ✅ Orchestrator top-level catch가 -v/--verbose/--log-level verbose/SYMPHONY_LOG_LEVEL 모두 인식
  • -v → orchestrator verbose 로깅 브리지 (foreground + daemon)
  • ✅ help 텍스트에 "stack traces" 문서화 + 스냅샷 갱신

직접 수행한 검증

  • pnpm build — 성공
  • 대상 테스트 53개 통과 (error-format / orchestrator index / start / help)
  • pnpm typecheck — 통과
  • pnpm lint — 통과
  • Smoke test (빌드된 @gh-symphony/core 직접 실행):
    • non-verbose → template_render_error 한 줄 ✔
    • verbose → 전체 stack + Caused by: 로 cause 체인 추적 ✔ (이슈가 요구한 동작 그대로)
  • 순환 cause 보호 로직([Circular cause]) 및 non-Error 입력 처리 확인 ✔
  • CI: Test ✅ / Container Smoke ✅

코드 품질 메모

  1. (비차단) 데몬 경로 verbose 전달 누락 — 인라인 코멘트 참조. 좁은 엣지 케이스(데몬 top-level uncaught 에러 한정, 서비스 레벨 verbose 로깅은 정상)라 머지를 막지 않습니다. 후속 한 줄 수정으로 마무리하면 좋습니다.
  2. orchestrator parseArgsstartsWith("--")startsWith("-") 변경으로 알 수 없는 single-dash 옵션이 이제 throw 되지만, 내부 전용 바이너리라 영향 없음.

핵심 기능이 이슈 요구사항대로 정확히 동작하고 모든 검사가 통과하므로 Approve 합니다. 위 비차단 항목은 선택적 후속 작업입니다.


Generated by Claude Code

options,
projectId,
parsed.logLevel,
parsed.logLevel ?? (options.verbose ? "verbose" : undefined),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검증 결과: 데몬 경로의 verbose가 자식 프로세스 top-level catch까지 전달되지 않습니다 (Codex P2 확인).

startDaemon은 자식을 gh-symphony repo start --log-level verbose 로 띄웁니다(line ~1087). 그런데 자식 CLI의 top-level catch(packages/cli/src/index.ts)는 hasVerboseFlag(process.argv.slice(2)) 로만 verbose를 켭니다. hasVerboseFlag-v/--verbose만 인식하고 --log-level verbose는 인식하지 않습니다.

로컬에서 확인:

hasVerboseFlag(["repo","start","--log-level","verbose"]) === false

따라서 gh-symphony -v repo start --daemon 으로 띄운 데몬이 startup 중 top-level 에러로 죽으면, 사용자가 -v를 줬는데도 데몬 로그에는 여전히 한 줄 메시지만 남습니다 — #396이 정확히 노리는 시나리오입니다.

범위는 좁습니다(서비스 레벨 verbose 로깅은 정상 전달되며, 영향은 데몬의 top-level uncaught 에러에 한정). 선택적/비차단 수정 제안: verbose 요청 시 자식 args에 --verbose도 함께 넘겨 자식의 top-level catch가 인식하도록 하면 됩니다.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5049ad9: startDaemon now passes --verbose to the child CLI when the parent global verbose option is set, alongside --log-level verbose. Added start.test.ts coverage asserting the daemon child argv includes both flags.

@moncher-dev moncher-dev force-pushed the feat/396-verbose-errors branch from 5049ad9 to e1ae786 Compare June 24, 2026 23:54
@moncher-dev moncher-dev merged commit 6b1b389 into main Jun 24, 2026
2 checks passed
@moncher-dev moncher-dev deleted the feat/396-verbose-errors branch June 24, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

-v/--verbose is a no-op and top-level errors discard stack/cause

3 participants